Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

7288: include WithdrawalRequestPredeployAddress in genesis configuration #7356

Conversation

Matilda-Clerke
Copy link
Contributor

@Matilda-Clerke Matilda-Clerke commented Jul 22, 2024

PR description

Allow configuration of WithdrawalRequestPredeployAddress in genesis configuration

Fixed Issue(s)

#7288

Thanks for sending a pull request! Have you done the following?

Locally, you can run these tests to catch failures early:

  • unit tests: ./gradlew build
  • acceptance tests: ./gradlew acceptanceTest
  • integration tests: ./gradlew integrationTest
  • reference tests: ./gradlew ethereum:referenceTests:referenceTests

Signed-off-by: Matilda Clerke <matilda.clerke@consensys.net>
Signed-off-by: Matilda Clerke <matilda.clerke@consensys.net>
Signed-off-by: Matilda-Clerke <matilda.clerke@consensys.net>
Copy link
Contributor

@siladu siladu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Might be nice to add a startup log when the address is different to default, could be a separate PR.

@Gabriel-Trintinalia
Copy link
Contributor

@macfarla Do you think we should have the same for Consolidation requests since we have it for deposits and withdrawals?

Copy link
Contributor

@macfarla macfarla left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

think it makes sense to use "withdrawal request contract" similar to "deposit contract" rather than "predeploy" unless there's a reason to keep that

@macfarla
Copy link
Contributor

@macfarla Do you think we should have the same for Consolidation requests since we have it for deposits and withdrawals?

yes i think so. created #7381

Signed-off-by: Matilda Clerke <matilda.clerke@consensys.net>
…ntractAddress

Signed-off-by: Matilda Clerke <matilda.clerke@consensys.net>
Signed-off-by: Matilda Clerke <matilda.clerke@consensys.net>
Signed-off-by: Matilda-Clerke <matilda.clerke@consensys.net>
Copy link
Contributor

@siladu siladu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changelog needs moving from 24.7.1 section to "next release"

Signed-off-by: Matilda Clerke <matilda.clerke@consensys.net>
@Matilda-Clerke
Copy link
Contributor Author

Good catch, thanks Simon. I didn't think of that

@Matilda-Clerke Matilda-Clerke merged commit 94b497e into hyperledger:main Jul 29, 2024
40 checks passed
gconnect pushed a commit to gconnect/besu that referenced this pull request Aug 26, 2024
…ion (hyperledger#7356)

* 7288: include WithdrawalRequestPredeployAddress in genesis configuration

Signed-off-by: Matilda Clerke <matilda.clerke@consensys.net>

* 7288: Update changelog

Signed-off-by: Matilda Clerke <matilda.clerke@consensys.net>

* 7288: Fix typo in new variable

Signed-off-by: Matilda Clerke <matilda.clerke@consensys.net>

* 7288: Rename withdrawalRequestPredeployAddress to withdrawalRequestContractAddress

Signed-off-by: Matilda Clerke <matilda.clerke@consensys.net>

* 7288: Update changelog to match recent changes

Signed-off-by: Matilda Clerke <matilda.clerke@consensys.net>

* 5098: Move changelog item to next release

Signed-off-by: Matilda Clerke <matilda.clerke@consensys.net>

---------

Signed-off-by: Matilda Clerke <matilda.clerke@consensys.net>
Signed-off-by: Matilda-Clerke <matilda.clerke@consensys.net>
Signed-off-by: gconnect <agatevureglory@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants